While I am absolutely in agreement with this in spirit, I don't think we will ever actually be able to remove the use of the non-new qualified variant. There is simply too much code out there that is using these classes without the new keyword. If I recall correctly, we tried to make this kind of change before to some other classes and it end up breaking a lot of people. So we need to be very careful on how we move forward, if we do choose to.

There is simply too much code out there that is using these classes without the new keyword.

Without deprecating them it's impossible to get rid of them. Runtime deprecation will create noise, but that's why we will make it a semver major. I personally don't see a downside of deprecating these. I think the timeline to remove this feature, is a different discussion.

+1 as I think this is necessary housekeeping, and not necessarily make it eol. As per standard, it needs to be doc deprecated first and then runtime deprecated.

I don't see a good reason to put the community under this kind of stress.

@mcollina If you are open to discussion and don't have hard no on this particular change, here's some:

• this has a huge impact on ecosystem and potentially leads the ecosystem to use modern javascript syntax
• the performance implications on initializing these classes (since there won't be ReflectConstruct and instanceof checks)
• modernization of the node.js codebase. eventually zlib code can be quite simplified if we used modern syntax. i recently did a similar work on cloudflare workers, and it was an eye opening experience for me.

@anonrig I was thinking about this PR (I don't know why, I don't even remember how I found it in the first place 😅), and if your goal is purely to move to ES6 classes while preserving behavior, you might be interested in this code snippet:

// extending this function allows providing a custom "this" value for classes
function Identity(x) { return x }
Identity.prototype = Object.prototype;

class InternalBaseClass extends Identity {
  constructor(param1, param2, customThis) {
    super(customThis);
  }
}

class InternalSubClass extends InternalBaseClass {
   constructor(opts, customThis) {
     super(opts, DEFLATE, customThis);
   }
}

// Replace the InternalSubClass constructor with one that can also be called without new
export function PublicSubClass(opts) {
  if (!(this instanceof InternalSubClass)) {
    return new InternalSubClass(opts, undefined);
  }
  return new InternalSubClass(opts, this);
}
Object.setPrototypeOf(PublicSubClass, Object.getPrototypeOf(InternalSubClass));
PublicSubClass.prototype = InternalSubClass.prototype;
PublicSubClass.prototype.constructor = PublicSubClass;

I didn't test performance.

Once Node.js ships decorators, it could become

// extending this function allows providing a custom "this" value for classes
function Identity(x) { return x }
Identity.prototype = Object.prototype;

class InternalBaseClass extends Identity {
  constructor(customThis, param1, param2) {
    super(customThis);
  }
}

export @allowCall class PublicSubClass extends InternalBaseClass {
   constructor(customThis, opts) {
     super(customThis, opts, DEFLATE);
   }
}

function allowCall(Class) {
  function Callable(...args) {
    return new Class(this instanceof Class ? this : undefined, ...args);
  }
  Object.setPrototypeOf(Callable, Object.getPrototypeOf(Class));
  Callable.prototype = Class.prototype;
  Callable.name = Class.name;
  Callable.prototype.constructor = Callable;
  return Callable;
}

I don't see a good reason to put the community under this kind of stress.

@mcollina Since the documentation-only deprecation landed, I want to move forward with this pull-request. I've tried to address your block on my comment: #54676 (comment). Did you had a chance to think over this, and maybe remove the block? We can always add tsc-agenda but I prefer to communicate before doing so.

If any other @nodejs/tsc members are interested in reviewing, we need 2 TSC approvals to land this since this is a semver-major pull-request.

Codecov Report

Attention: Patch coverage is 21.42857% with 22 lines in your changes missing coverage. Please review.

Project coverage is 87.59%. Comparing base (143aca7) to head (c9307a2).
Report is 3 commits behind head on main.

@@            Coverage Diff             @@
##             main   #54676      +/-   ##
==========================================
- Coverage   87.61%   87.59%   -0.02%     
==========================================
  Files         650      650              
  Lines      182945   182964      +19     
  Branches    35392    35393       +1     
==========================================
- Hits       160280   160267      -13     
- Misses      15917    15955      +38     
+ Partials     6748     6742       -6